Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

fix: No audio/video when in MLS call (WPB-6984) #2583

Merged
merged 1 commit into from
Mar 5, 2024
Merged

fix: No audio/video when in MLS call (WPB-6984) #2583

merged 1 commit into from
Mar 5, 2024

Conversation

alexandreferris
Copy link
Contributor

PR Submission Checklist for internal contributors

  • The PR Title

    • conforms to the style of semantic commits messages¹ supported in Wire's Github Workflow²
    • contains a reference JIRA issue number like SQPIT-764
    • answers the question: If merged, this PR will: ... ³

  • The PR Description

    • is free of optional paragraphs and you have filled the relevant parts to the best of your ability

What's new in this PR?

Issues

When in MLS call we would still call wcall_set_clients_for_conv

Causes (Optional)

wcall_set_clients_for_conv should be called only when conversation is Proteus

Solutions

Wrap calling of wcall_set_clients_for_conv only when conversation protocol is Proteus

Testing

How to Test

  • Have 2 accounts with E2EI + MLS enabled
  • Have a group with both accounts
  • Start a call
  • Accept/Join call from another user
  • Enable mic/video and everything should be working as expected.

@alexandreferris alexandreferris requested review from a team, yamilmedina, borichellow, vitorhugods, mchenani and e-lisa and removed request for a team March 4, 2024 16:49
@github-actions GitHub Actions
Copy link
Contributor

github-actions bot commented Mar 4, 2024 •
edited
Loading

Test Results

2 809 tests   2 783 ✔️  4m 46s ⏱️
       7 suites       26 💤
       7 files           0 ❌

Results for commit fe8f791.

♻️ This comment has been updated with latest results.

@codecov-commenter
Copy link

Codecov Report

All modified and coverable lines are covered by tests ✅

❗ No coverage uploaded for pull request base (release/candidate@010fbfd). Click here to learn what that means.

Additional details and impacted files 
@@                 Coverage Diff                  @@
##             release/candidate    #2583   +/-   ##
====================================================
  Coverage                     ?   58.28%           
  Complexity                   ?        7           
====================================================
  Files                        ?     1175           
  Lines                        ?    45851           
  Branches                     ?     4339           
====================================================
  Hits                         ?    26723           
  Misses                       ?    17184           
  Partials                     ?     1944

Continue to review full report in Codecov by Sentry.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 010fbfd...fe8f791. Read the comment docs.

@datadog-wireapp
Copy link

datadog-wireapp bot commented Mar 4, 2024 •
edited
Loading

Datadog Report

All test runs f3d568b 🔗

✅ 2 Total Test Services: 0 Failed, 2 Passed

Test Services
Service Name Failed Known Flaky New Flaky Passed Skipped Wall Time Test Service View
kalium-ios 0 0 0 2699 129 8m 0.1s Link
kalium-jvm 0 0 0 2821 123 8m 47.05s Link

vitorhugods
vitorhugods reviewed Mar 4, 2024
View reviewed changes
Copy link
Member

@vitorhugods vitorhugods left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Tests, please?

TestsPoliceGIF

vitorhugods
vitorhugods approved these changes Mar 5, 2024
View reviewed changes
Copy link
Member

@vitorhugods vitorhugods left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I just realised that CallManagerImpl is not testable at the moment.

✅✅✅✅✅

@MohamadJaara MohamadJaara merged commit 691f69d into release/candidate Mar 5, 2024
18 checks passed
@MohamadJaara MohamadJaara deleted the fix/no_audio_video_for_mls_calls branch March 5, 2024 07:49
github-actions bot pushed a commit that referenced this pull request Mar 5, 2024
@alexandreferris @github-actions
fix: only call wcall_set_clients_for_conv when conversation protocol …
0cae720 
…is proteus (#2583)
@github-actions github-actions bot mentioned this pull request Mar 5, 2024
Merged
4 tasks
github-merge-queue bot pushed a commit that referenced this pull request Mar 5, 2024
@github-actions @alexandreferris
fix: No audio/video when in MLS call (WPB-6984) (#2584)
92e1402 
* fix: only call wcall_set_clients_for_conv when conversation protocol is proteus (#2583)

* empty trigger commit

---------

Co-authored-by: Alexandre Ferris <[email protected]>
jschumacher-wire pushed a commit that referenced this pull request Mar 6, 2024
@github-actions @alexandreferris @jschumacher-wire
fix: No audio/video when in MLS call (WPB-6984) (#2584)
33b808e 
* fix: only call wcall_set_clients_for_conv when conversation protocol is proteus (#2583)

* empty trigger commit

---------

Co-authored-by: Alexandre Ferris <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@vitorhugods vitorhugods vitorhugods approved these changes

@MohamadJaara MohamadJaara MohamadJaara approved these changes

@yamilmedina yamilmedina Awaiting requested review from yamilmedina yamilmedina was automatically assigned from wireapp/android

@borichellow borichellow Awaiting requested review from borichellow borichellow was automatically assigned from wireapp/android

@mchenani mchenani Awaiting requested review from mchenani mchenani was automatically assigned from wireapp/android

@e-lisa e-lisa Awaiting requested review from e-lisa e-lisa was automatically assigned from wireapp/android

@Garzas Garzas Awaiting requested review from Garzas

Assignees

@alexandreferris alexandreferris

Labels
👕 size: S type: bug / fix 🐞
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@alexandreferris @codecov-commenter @MohamadJaara @vitorhugods